Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PS-13958] - Extend options for listing jobs #219

Merged
merged 3 commits into from
Apr 3, 2025

Conversation

hiiamelliott
Copy link
Contributor

Link to JIRA

PS-13958

What issue does this pull request solve?

Extends the query functionality of jobs_list() to include as many of the options available in Domino's REST API as possible.

What is the solution?

Added additional arguments and set their default values to match those in the REST API.

Testing

Tested by listing previous jobs in a project. As the function explicitly sends the default API values, it uses all of the query parameters by default.

  • Unit test(s)

Pull Request Reminders

References (optional)

@hiiamelliott hiiamelliott requested a review from a team as a code owner March 24, 2025 12:32
@hiiamelliott hiiamelliott mentioned this pull request Mar 24, 2025
2 tasks
domino/domino.py Outdated
project_id: str,
order_by: str = "number",
sort_by: str = "desc",
page_size: int = 3,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we keep the page_size in None as a default value? If this default value is enforced by Domino (I see the 3 in the old description) and changes in the future, we might want to also have that change applied seamless here.

What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can. I had doubts about hard-coding any of the API defaults for the same reason.
I guess page_size makes the biggest difference, because it limits how many results you see, whereas the others are just filtering and sorting them, right?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeap, I agree. The rest will probably be something that users would manually tweak if needed 👍

@ddl-s-rodriguez ddl-s-rodriguez merged commit e5a0461 into master Apr 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants